machine: Retrieve an unique identifier if one is known.#1095
machine: Retrieve an unique identifier if one is known.#1095dpgeorge merged 1 commit intomicropython:masterfrom
Conversation
1ba9531 to
12410fd
Compare
|
Thanks for the patch! I never knew about
I think it's worth removing that validation step, to save code size. |
unix-ffi/machine/machine/__init__.py
Outdated
| if len(data) == 32: | ||
| for byte in data: | ||
| if byte not in b"0123456789abcdef": | ||
| break |
There was a problem hiding this comment.
Suggest keeping the len(data) == 32 check but removing the hex char validation loop.
(If the file exists and has at least 32 bytes, it's highly likely to be a well formed hex number.)
unix-ffi/machine/machine/__init__.py
Outdated
| if byte not in b"0123456789abcdef": | ||
| break | ||
| # unhexlify might not be available | ||
| return bytes([int(data[i : i + 2], 16) for i in range(0, len(data), 2)]) |
There was a problem hiding this comment.
Suggest replacing len(data) with 32, since that 32 is validated above (and it'll reduce code size).
unix-ffi/machine/machine/__init__.py
Outdated
| return bytes([int(data[i : i + 2], 16) for i in range(0, len(data), 2)]) | ||
| except OSError as e: | ||
| if "ENOENT" not in str(e): | ||
| raise |
There was a problem hiding this comment.
Suggest to just do pass here in the except clause, ie silently ignore any error. That'll allow it to try the DBus file if anything fails in the etc file, it'll reduce code size, and also work correctly on targets that cannot render exceptions (ie MICROPY_PY_ERRNO is disabled).
12410fd to
845559d
Compare
|
Thanks for the review, I've removed the extra bits in question. The code still works as expected, and weirdly enough the size difference is now back to the initial iteration of this changeset (198 bytes, 317 - 119). |
dpgeorge
left a comment
There was a problem hiding this comment.
Thanks for updating, looks good.
This commit adds a proper implementation for "machine.unique_id" for selected systems, as opposed to returning a fixed value in every case. On a Unix system there are several incompatible ways to retrieve something that can be called an unique machine identifier. However, the only semblance of a specification comes from freedesktop.org, which says that a file called "/etc/machine-id" has to exist and must contain a 32-digits long hexadecimal identifier. Given that the current specification is the DBus unique identifier retrieval mechanism made official, if the identifier file is not found the code will attempt to read the file provided by DBus. These changes only apply to Linux and recent BSD systems. On other systems the old, fixed value for the machine identifier will be returned instead - to avoid breaking compatibility with existing code. The specification used in this commit is available at https://www.freedesktop.org/software/systemd/man/latest/machine-id.html. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
845559d to
75eb4aa
Compare
Summary
This PR adds a proper implementation for
machine.unique_idfor selected systems, as opposed to returning a fixed value in every case.On a Unix system there are several incompatible ways to retrieve something that can be called an unique machine identifier. However, the only semblance of a specification comes from freedesktop.org, which says that a file called
/etc/machine-idhas to exist and must contain a 32-digits long hexadecimal identifier - thus having a 128 bits unique identifier value.Given that the current specification is the DBus unique identifier retrieval mechanism made official, if the identifier file is not found the code will attempt to read the file provided by DBus.
These changes only apply to Linux and recent BSD systems. On other systems the old, fixed value for the machine identifier will be returned instead, to avoid breaking compatibility with existing code.
The specification used in this commit is available at https://www.freedesktop.org/software/systemd/man/latest/machine-id.html.
The manifest's version was bumped up by 0.0.1 since these changes only affect the behaviour of the module without making changes to the existing API.
Testing
This was tested on current MicroPython master, by adding
require("machine")toports/unix/variants/manifest.pyand rebuilding thestandardvariant of the interpreter.Then the output of
import machine; machine.unique_id()was checked against the contents of/etc/machine-idon the host system. The fallback was tested by changing/etcinto something else, and I/O error passthrough was tested by adding a directory that is not accessible by the running user to the front of the list of paths to check.Trade-offs and Alternatives
These changes add 198 bytes to the
machinemodule. Things could be made smaller by removing the identifier validation and skipping the DBus fallback.Preliminary identifier value validation in theory could be removed, since the "unhexlification" step will raise an exception if the input data is not a series of hexadecimal digits. The current behaviour is to ignore the data read from the current machine identifier file and try the next one in the list. If stopping the identification process on unexpected data at any point is acceptable rather than gracefully handling the condition, this could shrink down by 43 bytes.
However, since there is an existing specification and a known alternative path for the same data, maybe it's not a bad idea to keep those two bits of code in.
Generative AI
I did not use generative AI tools when creating this PR.